Add search, sort, filter, and windowing to the analysis catalog - #253
Add search, sort, filter, and windowing to the analysis catalog#253alex-rawlings-yyc wants to merge 8 commits into
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📝 WalkthroughWalkthroughAdds a localized Analysis Catalog panel with search, sorting, facet filters, tri-state morpheme filtering, zero-usage filtering, virtualized rows, and expanded test coverage. Jest mocks now support the required platform controls and icons. ChangesAnalysis Catalog
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to The catalog enhancements are otherwise mergeable, but the resize mock can accumulate keydown listeners when its target ref changes, leading to repeated event handling in affected tests or integrations. This is a bounded, localized follow-up item. Sequence Diagram(s)sequenceDiagram
participant CatalogUser
participant AnalysisCatalogPanel
participant CatalogQueryControls
participant CatalogFilterPopover
participant useRowWindow
CatalogUser->>CatalogQueryControls: enter search or choose sort
CatalogUser->>CatalogFilterPopover: choose filters
CatalogQueryControls->>AnalysisCatalogPanel: update query state
CatalogFilterPopover->>AnalysisCatalogPanel: update filter state
AnalysisCatalogPanel->>useRowWindow: pass queried rows
useRowWindow-->>AnalysisCatalogPanel: return visible row window
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Linked Issues checkExplanation The changes implement the objectives in issue Full details: Docstring CoverageExplanation Docstring coverage is 97.92% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 48 functions across 21 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
324afc3 to
cd98a29
Compare
378003a to
f4b84ce
Compare
3b2cdb2 to
440cfc9
Compare
The catalog panel listed every analysis in one fixed order with no way to narrow it, so a draft of any size was only navigable by scrolling. The query core already supported all of this; only the UI that varies it was missing. Search, sort, and filter state is ephemeral useState inside the panel. The panel is mounted only while open, so closing it clears the query — a filter that survived a reload would leave rows missing with nothing on screen saying why. Filters sit behind one control that reports how many are active, so a panel narrow enough to need filtering is not itself filled with them. All four groups ship: the facet-derived ones (books, part of speech, confidence, and each named feature), missing gloss, breakdown, and unused-only. Against today's data only books raises a control, since no write path records the others yet — the facets are rightly absent rather than offering a lone choice. Facets are derived from every row rather than from the rows a filter left standing, so a selection cannot collapse the facet that would widen it back. The new useRowWindow mounts a growing leading slice of the listing, extending as the end comes into reach and starting over when the query changes. It is deliberately not useSegmentWindow: a row list has no counterpart to the scripture reference that hook holds still, so it needs none of that geometry bookkeeping. A listing narrowed to nothing now says so, rather than reusing "No analyses recorded yet" and telling readers their draft is empty when they have merely mistyped. That message and the panel's original one both go through the platform EmptyState. Stubs the platform SearchBar, Select, MultiSelectComboBox, and EmptyState, each documenting where it diverges from the component it stands in for.
The sort option substituted the raw book code into "Most used in
{book}",
so the dropdown read "Most used in GEN" while the row column beside it
resolved the same book through Canon.bookIdToEnglishName and read "Uses
in
Genesis" — one book named two ways in one open panel.
Resolve the name once in the panel and pass it to both views, so the two
labels cannot disagree. CatalogQueryControls takes the resolved name
rather
than the code, which keeps it presentational and leaves book-name
resolution
in the panel.
The test rerendered through a bare InterlinearNavProvider rather than the PanelProviders root it mounted with. React saw a different element type at that position and remounted the provider, reinitializing the ref that holds the pending request — so the closing assertion found no request because none had survived the remount, not because navigating past EXO had abandoned one. Deleting the abandonment effect entirely left the test green. Rerender through PanelProviders instead, keeping the provider that owns the request mounted across both navigation steps. The test now fails with "EXO 3:14:8" when the abandonment effect is removed. Also lift the two collators out of the query memo. They were rebuilt on every keystroke in the search box, which changes the query but neither language tag.
Two identical declarations shadowed each other, and neither tsc nor ESLint covers __mocks__, so nothing flagged it.
The stub group seeded its layout from `defaultLayout` unconditionally, where the real group takes that prop only when it names exactly the panels mounted and discards it otherwise. A layout naming a closed panel therefore came back by itself here on the next mount, which upstream would have thrown away. That divergence hid the loader's restoring effect: the width it exists to reapply was already in the group's state before it ran, so the effect could be made a no-op with every catalog test still passing. Seed after the panels have registered instead, matching on the count as the real group does. Cover the effect with a test that mounts the group closed, leaving it knowing only of the view so the stored layout reaches it only by being applied as the catalog's panel joins. Disabling the effect now fails five tests.
The window reset exists for a changed query: a reader who narrows a listing is looking at a new list, not further down the old one. It was keyed on the rows array's identity instead, which is only a proxy for that — and a proxy that also turns over on any edit to the underlying analysis. A gloss approved in the view beside an open catalog therefore collapsed a deeply scrolled list back to its first chunk, throwing the reader to the end of forty rows while the sentinel re-extended beneath them. useRowWindow now takes the query itself and compares that by reference, so the hook does what its own doc comment already claimed. Also withhold the query controls from a draft that has recorded nothing, where a search box, sort and filter popover narrow an empty listing and the popover in particular is an invitation to a dead end. The gate reads the draft rather than the queried rows, so a query that matched nothing keeps the controls that are the only way to widen it back.
A feature value is free text, so it may be the empty string. The filter offered such a choice under its own spelling, which the platform control can neither carry as a value nor show as a label: the choice appeared as a blank, unclickable row. Give it a sentinel of its own, alongside the one the absent choice already uses, and a localized label to be read under. Route the three places that spelled a choice for the control through one function, so they cannot disagree about the two choices that need spelling. Latent for now, no write path recording feature values yet.
440cfc9 to
d85e9e3
Compare
Closes #231.
The catalog listed every analysis in one fixed order with no way to narrow it, so a draft of any size was navigable only by scrolling. The query core (#192, PR #211) already backed all of this —
applyCatalogQuery,deriveFacets,CatalogSort, andCatalogFilterswere built and tested. This is the UI that varies them: the panel's fixed query literal becomes state the controls drive, and nothing in the query core changed.Search, sort, and filter state is ephemeral
useStateinside the panel. The panel is mounted only while it is open, so closing it clears the query by construction rather than by a reset effect — a filter that survived a reload would leave rows missing with nothing on screen saying why. Open/closed and width stay tab-scoped inuseWebViewState.Filters sit behind one control
A panel narrow enough to need filtering should not itself be filled with filter controls, so all of them live in a popover whose trigger reports how many are active.
All four groups ship. Against today's data only books ever raises a control: nothing writes part of speech, confidence, or features yet, so
deriveFacetscorrectly yields nothing for them and no control appears. Used nowhere ships too, and matches nothing until PT9 import (#150) or the catalog's own delete/merge paths land —detachTokenAnalysisLinkdrops a payload with its last link, so no current write path can produce a zero-usage row. Both are the outcome #231 decided on and both are tested for, not gaps.Facets are derived from every row the draft holds rather than from the rows a filter left standing. A facet judged against its own selection's survivors would collapse to that selection, leaving nothing on screen to widen it back by.
Carrying no value is a choice of its own
CatalogFacetslists the absent value asundefined, which is what lets a reader ask which analyses are still missing a field as readily as which carry a given value. The platformMultiSelectComboBoxspeaks strings alone, so that choice needs a spelling:\u0000untagged, a leading NUL being one no part of speech, confidence level, or feature value can collide with. Values are read back through a map rather than compared against the sentinel, soundefinedis recovered as the choice it is. Worth a look — it is the one place the control's vocabulary and the filter's diverge.Books is the exception: a usage names a book by construction, so that facet never offers an untagged choice, and the selection is filtered before it reaches
CatalogFilters.Windowing
useRowWindowmounts a growing leading slice of the listing and extends by a chunk each time an end-of-list sentinel comes within reach. Grow-only and anchored to nothing — it never culls from the top and never adjusts the scroll position. Deliberately notuseSegmentWindow: that hook is anchored around a scripture reference, and a row list has no counterpart to hold still, so it needs none of that geometry bookkeeping.Two details worth the reviewer's attention:
rowsis a different array, adjusted during the render that first sees the new listing rather than in an effect afterwards — an effect would let one frame paint the new rows at the old, grown count before shrinking back. Keyed on array identity rather than length, because a query can narrow a listing to a different set of rows of the same size.IntersectionObserverreports only intersection transitions: after an extend the sentinel node is unchanged and may still sit inside the arming margin, where a stale observer would stay silent however far the reader scrolls. A fresh observer re-delivers the current state, extending once per delivery until the sentinel is pushed clear.Not in the issue: a listing narrowed to nothing says so
Reusing "No analyses recorded yet" for a query that matched nothing would tell readers their draft is empty when they have merely mistyped, and send them looking for lost work. There is now a second message for that case, and both go through the platform
EmptyStaterather than a hand-rolled paragraph.Search semantics are unchanged and accepted as-is
applyCatalogQuerymatches the whole trimmed, folded query as a single substring against a per-row blob. So a multi-word query never matches across fields, there is no match weighting, and ordering is the chosen sort key alone. The placeholder promises "Search forms and glosses" and nothing more. Multi-term search, relevance ranking, and highlighting are query-core work, not UI work, and separate issues if wanted.Mocks
Stubs the platform
SearchBar,Select,MultiSelectComboBox, andEmptyState, each documenting where it diverges from the component it stands in for — notably thatMultiSelectComboBoxresolves an entry by label as the real component's own select handler does, so a stub test cannot pass on a collision the real component would drop.Testing
Covers all eight behaviors #231 lists. Full suite passing, 100% coverage, lint clean.
This change is
Summary by CodeRabbit
New Features
Tests